fix(restart): say a restart Harper accepted is not back yet, never that it failed - #1899

Rebased and ported (2026-10-12). The stack is now linear on current stage with no merge commits, and this PR's loop changes are ported onto the cluster restart and rolling config loops #1861 and #1867 rewrote (getClusterRestartTargets, the 5.2+ set_configuration fan-out, the failed-cluster-load toast, Cancel during the instance check). The design is unchanged; what the port had to decide is under Port onto #1861/#1867, and four review rounds after it added the fixes listed under Review after the port.

⊙ Problem

A restart Harper accepted is reported as failed when the target is slow to answer again: restartInstance sends the restart, then watches for the target with user_info, and both steps rejected the same promise. So the Restart button toasts "Failed to restart instance.", the cluster restart and rolling config update count the member as not restarted, and the chat tools return success: false. Each one invites a second restart of a target that already went down once (#1895).

❓ Your call: is this the right size for the fix? Chosen: classify at the producer, keep watching for a second probe schedule, and stop the one-at-a-time loops at a member that is not back. The smaller alternative is a copy-only change ("Failed" → "couldn't confirm") on every caller, which would drop the refused/accepted distinction a user needs (a refused restart is safe to retry; an accepted one is not). Reversible by reverting this PR.

💡 Solution

Harper's answer to the restart request is its only verdict on the restart. restart returns RESTART_RESPONSE on the main thread and schedules the teardown 50ms later "so we give the HTTP thread a chance to return a response" (harper bin/restart.ts:116-188; a non-object result is wrapped as { message } at server/serverHelpers/serverUtilities.ts:163-165). restart_service runs through executeJob and answers { message: 'Restarting http', job_id } once its job thread has been launched (serverUtilities.ts:590-617, server/jobs/jobRunner.ts:92-94); a bad service is a 400 from jobs.ts:130-133. So a 2xx means accepted, and the probe that follows only tells "back" from "not back yet". (Read-only harper checkout at v5.3.1-158-g16e5c663c.)

• restartInstance now rejects with the restart request's own error when that request failed (refused, or unanswered, which fix(errors): say a write's outcome is unknown when it got no verdict, not that it failed #1897's "outcome unknown" handles), and wraps only a failure after the POST resolved in the new RestartUnconfirmedError (cause: the last probe failure; its message keeps the probe's text so shouldKeepEvent filters it as before).
• The watch keeps going past the usual recovery time: one 10s settle, then two schedules of 12 probes, calling onSlowToReturn once between them so callers say "Still restarting" instead of giving up. The probes skip the gateway replay, the restart POST gets an explicit 60s timeout, and RESTART_MARK_TTL_MS is derived from the same constants, so no restart mark lapses mid-watch.
• describeError maps the new error to Not back yet: "Harper accepted the restart, but Studio couldn't confirm it came back. It may still be starting, so check its status before you restart it again." shouldToastOwnFailure keeps the button from adding "Failed to restart" beside it.
• The cluster restart and rolling config update stop at a member that never comes back rather than restart the next one, and report the members they left alone. Both also stop at a member whose restart got no verdict once a follow-up watch on it gives up too (review rounds 1 and 2).
• The chat tools keep success: true (their action was carried out) with copy telling the agent not to restart, or create the app, again.

Also fixed: both chat tools now replace their loading toast on any failure; before, "Restarting HTTP service..." / "Creating from template..." kept spinning, since a loading toast never dismisses itself.

⚠️ Look hardest: the producer boundary, where only a failure after the POST resolved becomes RestartUnconfirmedError; the shared TTL that has to outlast the whole watch; and, new with the port, where the config update's stop sits against the fan-out.

🔀 Port onto #1861/#1867

#1861 and #1867 replaced the loops this PR edited: both now plan their targets through getClusterRestartTargets, report a failed cluster load, and honor a Cancel clicked during the instance check; the rolling config update also fans set_configuration out from its first instance on Harper 5.2+. The old loop code is gone; this PR's intent is applied to the new code:

• Cluster restart (useRestartClusterClick.tsx): the stop, failed count, "Waiting On Instance" toast and shared TTL sit inside fix(clusters): Restart Cluster reports a failed cluster load instead of staying disabled #1867's load-failure try and after its Cancel check, so a failed load still reports that nothing was restarted and a Cancel during the check still stops before the restart.
• Rolling config update (useRollingConfigUpdate.tsx): the stop breaks before the fan-out block, so a canary that accepted the restart but is not back is never fanned out from: it never proved it can run the new configuration, which is why the fan-out waits for a canary at all. A stop at a later member, after the fan-out, keeps feat(config): fan cluster config saves out with replicated set_configuration on Harper 5.2+ #1861's note that the members left alone may already have the new configuration saved, in the uncertain-outcome toast and in the one that outranks Cancel.
• Tests live in the loops' own test files, which feat(config): fan cluster config saves out with replicated set_configuration on Harper 5.2+ #1861/fix(clusters): Restart Cluster reports a failed cluster load instead of staying disabled #1867 created, instead of the separate *.unconfirmed.test.tsx files this PR used to avoid add/add conflicts with them. They run through the real getClusterRestartTargets and plan, with restartInstance mocked to reach each outcome.

❓ Your call: after a fan-out, a stop at a member that is not back says "N other instances were not updated, to keep the cluster available" and then "Instances that were not restarted may already have the new configuration saved". Both are true (not restarted, possibly saved), but "not updated" reads slightly stronger than the fan-out allows. Rewording it to "not restarted" for the config update is a one-word change in describeNoReturn; I left the shared helper's verb alone.

⚖️ Alternatives

The planning review (codex) returned better-alternative-exists; its variant was adopted: keep the producer-owned typed error, but stop the rolling loops on it, keep watching inside one deadline, and give the chat tools a non-contradictory result.

• Classify in each caller by inspecting which request failed: five call sites re-deriving it from serialized request data, and the global mutation toast could not know the error came from a restart at all.
• Resolve instead of reject ({ data, answeredAgain: false }): fails open, since any caller not updated, and the mutation's global handler, would report an unconfirmed restart as "restarted!".
• Just lengthen the window: moves the edge (compaction on start, bin/restart.ts:135-137, is unbounded) without changing what the edge says.

❓ Your call: the loops now stop at a member that accepted the restart and never came back (cluster restart, rolling config; adopted from the planning review's blocker: carrying on can leave a two-member cluster with no member serving). The cost: re-clicking Restart Cluster restarts every member again, including those already done. The alternative is the old carry-on. Reversible by deleting the breaks.

❓ Your call: the watch is now about twice as long (≈3.5 → ≈7 min before "Not back yet"), with a "Still restarting" notice at the halfway point, and the Restart button stays disabled throughout. Shorter means more false "not back" stops in the loops; longer means a longer spinner.

❓ Your call: not adopted, against every review round: counting a probe answered 401/403 as the target being back. Through central manager's proxy a 403 is CM refusing the caller (notAllowed() in central-manager src/resources/hdbInstance/HDBInstance.js:415-422) before it contacts the instance, so it says nothing about the target (recorded in DESIGN.md). The copy says Studio "couldn't confirm it came back", which stays true for any failed probe. Treating a direct client's 401/403 as back would need a direct-vs-proxied split in the probe.

❓ Your call: for an unanswered restart (#1897's outcome-unknown case), both loops now wait on that member and stop there if the follow-up watch gives up too, as for a member that is not back (review rounds 1 and 2; before, the cluster loop discarded the watch's rejection and the config update did not wait at all). The config update's case is the lower-stakes one: Harper replaces HTTP workers an eighth at a time with the replacements started first (harper server/threads/manageThreads.js:1403-1419), so a member normally keeps answering. The cost is a wait of up to about seven minutes per unanswered member. Reversible by dropping the two waits.

❓ Your call: a Cancel clicked while a member's set_configuration is in flight now stops before that member's restart, and the Cancelled report says the configuration may be pending there (raised by codex in rounds 1 and 2; the adjudicator called finishing the member in flight defensible too). This was #1867's ordering, so it changes merged behaviour. Reversible by deleting that break.

🔧 Changes

• src/integrations/api/instance/status/restartUnconfirmed.ts (new): the error class, the Not back yet copy, and describeUncertainRollingOutcome, which both loops use for their final toast when an outcome is unknown: confirmed count, members that failed, unanswered writes (fix(errors): say a write's outcome is unknown when it got no verdict, not that it failed #1897's sentence), the member not back plus the ones left alone (worded for a no-verdict member too), and the members a Cancel left.
• src/integrations/api/instance/status/restartInstance.ts: the typed rejection, the two-schedule watch (a callback that throws is logged and the watch carries on), probes without gateway replay, the explicit request timeout, and the derived mark TTL.
• src/react-query/queryClient.ts: describeError and shouldToastOwnFailure recognise the new error.
• src/hooks/useRestartInstanceClick.ts: marks with the shared TTL and turns its toast into "Still restarting" halfway through the watch.
• src/hooks/useRestartClusterClick.tsx: shared TTL for both marks, a "Waiting On Instance i of n" toast, the stop, a failed count, the stop at a no-verdict member whose watch gave up, an uncertain outcome that outranks Cancel and names the members Cancel left, and progress toasts that stop offering Cancel once it was clicked.
• src/hooks/useRollingConfigUpdate.tsx: the same waiting toast, stop before the fan-out, failed count, wait on a no-verdict restart, and stop if it never answers, Cancel honored between the write and the restart, Cancel precedence and the same progress toasts; it marks nothing, so there is no TTL to change.
• Chat tools: describeToolError.ts adds toastToolFailure; restartHTTPService/execute.ts and createApp/execute.ts return success: true with no-retry copy for an unconfirmed restart, and replace their progress toast on every failure, including the app-creation one.
• src/features/instance/operations/index.tsx: the Operations console's restart mark uses RESTART_MARK_TTL_MS instead of its own 5 minutes, since its waitUntilAnsweringAfterRestart watch is now longer; its test's module mock gains the constant.
• DESIGN.md: a new section recording the verdict rule, the shared deadline, the successful-probe-only rule, the loop stop, the no-verdict stop, Cancel, and how the stop meets the 5.2+ fan-out, and the chat-tool contract.

✅ Verification

End-to-end route: integration-style tests through the real restartInstance, a real instance client (getInstanceClient with a fake adapter), and the real React Query mutation error handler; the loops are exercised through their real target planning with restartInstance mocked to reach each outcome. Not run against a live instance.

• Fails on base, passes here: the Restart-button test on base a35e2bed0 showed the bug's signature, two toasts Error / timeout of 3000ms exceeded and Error / Failed to restart instance. for a restart Harper answered 200; with the fix it shows one Not back yet toast and dismisses the loading toast. A second test holds the probe open past the first schedule and checks the mark is held (still held six minutes in, past the old TTL), the button pending, "Still restarting" shown, then success once it answers.
• restartInstance.test.ts drives a real instance client: an accepted restart that never comes back rejects with RestartUnconfirmedError after exactly 24 probes on the 10s + 23×15s schedule, a slow target is still watched, the request's own 60s bound plus every probe and every wait fit inside RESTART_MARK_TTL_MS, refused / gateway / no-answer POSTs keep their own error, and RUM filtering is unchanged for wrapped timeouts and network errors.
• useRestartClusterClick.test.tsx: stops at a member that is not back and names the ones left alone, goes on once a slow member answers (holding its mark and the cluster's past the old five minutes), still carries on past a refusal, names refused members beside the one not back, keeps the warning when Cancel is clicked, and still says Cancelled when nothing was uncertain. From the review rounds: a no-verdict member whose watch also gives up stops the loop, a Cancel after one names the members it left, and no toast after a Cancel offers Cancel again.
• useRollingConfigUpdate.test.tsx: the same stop, refusal and Cancel cases, plus the port's two, a canary that is not back is never fanned out from and a stop after the fan-out keeps the pending-configuration note, and from the review rounds the stop at a no-verdict restart, no wait after a no-verdict write, Cancel during the write and no second Cancel. restartTools.test.ts pins both chat-tool results and toasts; queryClient.test.ts pins the shared copy.
• Mutation checks on the ported code, each guard reverted locally and run against its tests: all 43 went red. The first pass found three guards no test held (the request's explicit timeout, which only shows with the client's same 60s default cleared; the marks' TTL, since sleep is mocked; the config update's failed count); each got a test in test(restart): pin … and then went red. Covered: the producer's wrap, timeout, probe replay opt-out, slow callback and its guard, second schedule and TTL budget; describeError and shouldToastOwnFailure; the button's TTL and toast; both loops' stop, classification, waiting toast, Cancel precedence, failed count, Cancel count, no-verdict stop and its copy, and progress toasts without Cancel; the cluster loop's two marks; the config update's pending note, wait gate, Cancel-during-write and its note; the shared failed sentence; and the chat tools' branches and toast.
• Gates on the head: npx vitest run exit 0 (522 files, 5033 passed, 1 expected fail, 11 skipped; the pre-commit hook also ran the full suite on every commit), npx tsc -b exit 0, npx oxlint --format stylish . exit 0, npx dprint check exit 0, all on Node 24.21.0.

Stack: based on #1897 (claude/1890-outcome-unknown-writes) → #1891 → #1882 → stage, all linear on current stage.

🔁 Review after the port

Four rounds, each with codex (graded), gemini and the Harper domain adjudicator, every one independent=true. The Cursor legs contributed nothing: round 1's grok leg exited 143, and every other Cursor attempt failed no-receipt because the 1Password SSH agent refused its git fetch; rounds 3 and 4 pruned them as narrow deltas. Round 4 (delta, at the head): codex LGTM, adjudicator Surviving-Findings: none.

Taken

• Round 1, codex major (kept major): the cluster loop waited on a member whose restart got no verdict, then discarded the watch's rejection and restarted the next member; a 504 after the restart ran, plus a member that stays down, left a two-member cluster with nobody serving. It now stops there, saying Studio couldn't confirm that member came back either.
• Round 1, adjudicator nit: a Cancel after a no-verdict member produced "Outcome unknown" without saying Cancel skipped the rest. Both loops now name the members Cancel left.
• Round 2, codex major twice (adjudicated minor, "fixable in the stack"): the config update moved on 3s after an unanswered restart_service, so a configuration that broke that member could reach the next one. It now waits and stops the same way; an unanswered set_configuration does not wait, since nothing restarted that member.
• Round 2, codex major in rounds 1 and 2 (adjudicated minor, pre-existing in fix(clusters): Restart Cluster reports a failed cluster load instead of staying disabled #1867): a Cancel during set_configuration still restarted that member. Now stops before the restart, and says the configuration may be pending (❓ above).
• Rounds 2 and 3, adjudicator minor / codex minor: after a Cancel, the slow-return, no-verdict and "carry on momentarily" toasts came back with a second Cancel button that did nothing new. One helper per loop now drops the action and says no other instance will be touched.
• Every round, comment nits: narrating test comments and helper JSDoc dropped, restartInstance's JSDoc trimmed to its contract, one flag comment removed.

Rejected on evidence

• "The watch outlives the progress toasts' durations" (codex, round 1): sonner 2.0.8 never starts an auto-close timer for a loading toast (dist/index.mjs:644), which the repo already relies on (useRestartInstanceClick.ts). The adjudicator dropped it as factually wrong.
• "?? drops the pending-configuration note" (gemini, round 1): the note is appended outside the parenthesised (uncertain?.description ?? …); a test pins it and its mutation went red.
• "A second watch after success holds the mark 10s" (gemini, rounds 1 and 3): holdUntilAnswering is set only in the catch, for a request that got no verdict.
• "axiosRetry resends the restart POST" (gemini, round 2): the POST is sent once; axiosRetry wraps only the probes, and fix(api): replay a gateway error only for reads, so a write is never sent twice #1891's replay never repeats a write.
• "Cancel aborts the watch into RestartUnconfirmedError" (gemini, round 2): Cancel only sets a flag; no signal reaches the watch.
• Gemini's remaining comment nits: the comments kept state an invariant or a contract, as the adjudicator ruled.

The harness's "state-trace coverage gap" on the new Cancel count was checked by hand: a member lands in none of restarted / failed / unanswered only when Cancel stopped the loop before its restart (or after its check), and the count is zeroed when the loop stopped at a member that is not back. Both loops' Cancel-count tests pin it, and its mutation went red.

Closes #1895

_{🤖 Generated with Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

Related PRs: #1897 overlaps (this PR's base), #1882 overlaps (base of the stack; shares the restart TTL), #1867 overlaps (merged; reworked both restart loops, ported onto here), #1861 overlaps (merged; reworked useRollingConfigUpdate and added the fan-out the stop now gates)
Complexity: medium

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; blocked=cursor-grok(failed),cursor-composer(no-receipt),cursor-muse(no-receipt); declined=cursor-kimi; rounds=4; full=1 @ 6cd6fbc}

_{Review-Attention: read ~6m (decisions: canceled-progress-copy, progress-helper-duplicated, cancel-mid-write-stops-before-restart, availability-stop-outranks-cancel, chat-tool-success-true, watch-length-8min, do-less-alternative) @ 6cd6fbc}